Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to ghcr.io #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Switch to ghcr.io #8

wants to merge 2 commits into from

Conversation

tedivm
Copy link

@tedivm tedivm commented Feb 21, 2022

The new container registry supersedes the one used here. The old one is buggy- it gets "unknown blob" errors occasionally and while Github says they'll fix that they also keep pushing people towards their new system.

The new container registry [supersedes](https://github.blog/2020-09-01-introducing-github-container-registry/) the one used here. The old one is buggy- it gets "unknown blob" errors [occasionally](https://github.community/t/random-unknown-blob-error-when-pushing-to-docker-github-package-repository/16725/38) and while Github says they'll fix that they also keep pushing people towards their new system.
@g105b
Copy link
Member

g105b commented Feb 21, 2022

Hey, thanks for the PR. I'll check it out later, but there is was a reason the old package repository was used... At the time of this repo being created, it was impossible to use ghcr to cache the builds within your own GitHub user's profile (without having to create and inject a personal access token).

Apparently this was a known limitation that was being worked on, so let's see how this works. Fingers crossed!

@tedivm
Copy link
Author

tedivm commented Feb 21, 2022

Hmm- I didn't test this to be honest, but it's possible that a token will still need to be passed through. If so it won't need to be a personal access token, rather it'll be the one passed to the github action by default. I really can't remember if it is required or not though.

@g105b
Copy link
Member

g105b commented Feb 21, 2022

You're right - this wasn't a PAT that was required, but at the time the token wasn't exposed in the test runner so it wasn't possible. I'm sure this has been fixed by now, but I'll give it a proper test when I get some time in the next couple of days.

@tedivm
Copy link
Author

tedivm commented Feb 21, 2022

It is exposed to the test runner now, but I don't believe it's exposed to the actions by default- the parent action has to pass it in as a variable. It may make sense to make it optional- if the token is passed in use ghcr, if not continue with the current behavior. The only real downside to the current behavior is the "unknown blob" error, and that only occurs on write (downloading already compiled containers works fine).

@g105b
Copy link
Member

g105b commented Feb 22, 2022

That's a good idea - progressive enhancement where the variable is set, usual behaviour where not. I've had the "unknown blob" get in my way a fair few times, so I'd be happy to see a fix.

I'll report back once I've had chance to test things on this branch. Cheers.

@tedivm
Copy link
Author

tedivm commented Feb 23, 2022

Sounds good- I can also give it a PR with optional switch once the weekend hits.

@g105b
Copy link
Member

g105b commented May 15, 2022

Hey @tedivm , just keeping you in the loop - I'm testing this today, hopefully it will just work as we expect, but I'll let you know the outcome here.

g105b added a commit to php-actions/example-composer that referenced this pull request May 15, 2022
@g105b
Copy link
Member

g105b commented May 15, 2022

Hmm, it seems like the same issue as before - to push to ghcr.io the developer needs to supply their Github PAT in an environment variable which isn't ideal. I wonder if this can be circumvented?

See the failing test here: https://github.com/php-actions/example-composer/runs/6441223120?check_suite_focus=true

image

@g105b
Copy link
Member

g105b commented Sep 7, 2022

@fagai regarding your PR on #10
and @tedivm regarding this PR.

It looks like I need to decide what to do for the change to ghcr.io

Has anyone got any ideas of how we can allow pushing to ghcr.io without requiring developers to create a PAT for every repo? It just worked without any issues with docker.pkg.github.com

@fagai
Copy link

fagai commented Sep 7, 2022

@g105b
sorry, It seems that the current incident is the cause.
https://www.githubstatus.com/incidents/d181frs643d4

@g105b
Copy link
Member

g105b commented Sep 7, 2022

Oh OK, thanks for clarifying - I still think it would be good to look into switching over, but I just don't know how to handle the requirement of developer PATs.

@tedivm
Copy link
Author

tedivm commented Sep 7, 2022

Pushing to GHCR does not require a PAT. It does require you to either set the organization permissions up to allow actions to write, or to explicitly allow the individual workflow to write.

If you look at my multi-py workflows I'm not using a PAT to push.

@tedivm
Copy link
Author

tedivm commented Sep 7, 2022

In your organization settings make sure write access is available-

Screen Shot 2022-09-07 at 8 32 38 AM

If you don't want to do that you can set the permissions on each workflow.

To quote Github-

To authenticate to the Container registry (ghcr.io) within a GitHub Actions workflow, use the GITHUB_TOKEN for the best security and experience. If your workflow is using a personal access token (PAT) to authenticate to a registry, then we highly recommend you update your workflow to use the GITHUB_TOKEN.

https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry#authenticating-to-the-container-registry

@g105b
Copy link
Member

g105b commented Sep 7, 2022

Oh that's great. This functionality has changed massively since I originally made these repos. php-actions was an early adopter of Github Actions, and I regret not waiting for things to become a bit more stable, because of the amount of time I've had to dedicate to changes over the years.

I'll look into it later, I hope this resolves things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants